Ignite 4756 #3

vadopolski · 2017-07-04T13:51:13Z

No description provided.

dream-x · 2017-07-04T13:55:41Z

...re/src/main/java/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunction.java

@@ -83,7 +84,7 @@
    private int parts;

    /** Mask to use in calculation when partitions count is power of 2. */
-    private int mask = -1;
+    private transient int mask = -1;


а зачем transient?

dream-x · 2017-07-04T13:55:58Z

...re/src/main/java/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunction.java

@@ -493,9 +496,91 @@ private static long hash(int key0, int key1) {
            assignments.add(partAssignment);
        }

+	    List<List<Integer>> dist = freqDistribution(assignments, nodes);
+	    printDistribution(dist);


отступы

dream-x · 2017-07-04T13:57:43Z

...re/src/main/java/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunction.java

+
+			for (int j = 0; j < backups; ++j) {
+				if (nodeMaps.get(j).get(node) == null)
+					byBackups.add(0);


Вообще почему у тебя отступы по 8 пробелов? 4 пробела во всем проекте

Не понял, по 4-ре вроде везде

присмотрись но тут 8))

восемь да, это жен цикл в цикле

dream-x · 2017-07-04T13:57:55Z

...re/src/main/java/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunction.java

+
+            log.info("");
+        }
+


убрать строку

какую?

под которой этот коммент

dream-x · 2017-07-04T13:58:30Z

...a/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunctionSimpleBenchmark.java

@@ -351,6 +351,24 @@ private double chiSquare(List<List<Integer>> byNodes, int parts, double goldenNo
    /**
     * @throws IOException On error.
     */
+    public void testImprovedLog() throws IOException {
+        int[] nodesCnts1 = {4};


dream-x · 2017-07-04T13:58:39Z

...a/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunctionSimpleBenchmark.java

+    public void testImprovedLog() throws IOException {
+        int[] nodesCnts1 = {4};
+
+        AffinityFunction aff3 = new RendezvousAffinityFunction(true, 1024);


dream-x · 2017-07-04T13:58:55Z

...a/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunctionSimpleBenchmark.java

+
+        AffinityFunction aff3 = new RendezvousAffinityFunction(true, 1024);
+
+        for (int nodesCnt1 : nodesCnts1) {


dream-x · 2017-07-04T13:59:11Z

...a/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunctionSimpleBenchmark.java

+            List<ClusterNode> nodes0 = createBaseNodes(nodesCnt1);
+
+            assignPartitions(aff3, nodes0, null, 2, 0).get2();
+            List<List<ClusterNode>> lst0 = assignPartitions(aff3, nodes0, null, 2, 1).get2();


dream-x · 2017-07-04T13:59:17Z

...a/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunctionSimpleBenchmark.java

+            assignPartitions(aff3, nodes0, null, 2, 0).get2();
+            List<List<ClusterNode>> lst0 = assignPartitions(aff3, nodes0, null, 2, 1).get2();
+
+            List<List<Integer>> dist0 = freqDistribution(lst0, nodes0);


dream-x · 2017-07-04T17:16:44Z

...a/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunctionSimpleBenchmark.java

+        for (int nodesCnt : nodesCnts) {
+            List<ClusterNode> nodes0 = createBaseNodes(nodesCnt);
+
+            assignPartitions(aff, nodes0, null, 2, 0).get2();


dream-x · 2017-07-04T17:17:28Z

...re/src/main/java/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunction.java

+     * @return Frequency distribution: counts of partitions on node.
+     */
+	private static List<List<Integer>> freqDistribution(List<List<ClusterNode>> lst, Collection<ClusterNode> nodes) {
+		List<Map<ClusterNode, AtomicInteger>> nodeMaps = new ArrayList<>();


1 пробел 1 символ = посчитай что у тебя))

не понимаю, напиши плз как должно быть

dream-x · 2017-07-04T17:17:51Z

...re/src/main/java/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunction.java

+        log.info("#### NODES:" + nodes + " nodes\n");
+
+        for (List<Integer> byNode : byNodes) {
+


убрать строку

dream-x · 2017-07-04T17:19:55Z

...re/src/main/java/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunction.java

+
+            nr = 0;
+
+            for (int w : byNode) {


а почему переменная называется w может лучше weight смотря что у тебя оно обозначает

dream-x · 2017-07-04T17:20:58Z

...re/src/main/java/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunction.java

+        int nr = 0;
+        int node = 0;
+
+        log.info("#### NODES:" + nodes + " nodes\n");


log.info("#### NODES:" + nodes + "\n"); зачем 2 раза повторять одно и тоже слово

Исправлено

dream-x · 2017-07-04T17:29:06Z

...re/src/main/java/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunction.java

+                nr++;
+            }
+
+            log.info("");


а вот это зачем?

dream-x · 2017-07-04T17:29:52Z

...re/src/main/java/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunction.java

 import org.apache.ignite.resources.LoggerResource;
 import org.jetbrains.annotations.Nullable;
+//import org.apache.ignite.logger.log4j.Log4JLogger;


а это почему осталась?

Improved Java Doc and Code Style.

dream-x · 2017-07-20T14:17:54Z

...re/src/main/java/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunction.java

+
+    /**
+     * @param nodesParts Frequency distribution.
+     * @param localNodeID


Почему нет описание переменной и дальше тоже

dream-x · 2017-07-20T14:18:02Z

modules/core/src/main/java/org/apache/ignite/cache/affinity/AffinityFunctionContext.java

+    /**
+     * Gets node name.
+     *
+     * @return Cache name.


а тут id

Improved Java Doc, Code Style and Abbreviation.

dream-x · 2017-07-24T11:26:23Z

...rc/main/java/org/apache/ignite/internal/processors/affinity/GridAffinityAssignmentCache.java

@@ -280,21 +282,26 @@ public void onReconnected() {

        List<List<ClusterNode>> assignment;

+        GridAffinityFunctionContextImpl afCtxt = new GridAffinityFunctionContextImpl(sorted, prevAssignment,


Давай вернем как было в мастере.

dream-x · 2017-07-24T11:26:39Z

...rc/main/java/org/apache/ignite/internal/processors/affinity/GridAffinityAssignmentCache.java

        if (prevAssignment != null && discoEvt != null) {
            boolean affNode = CU.affinityNode(discoEvt.eventNode(), nodeFilter);

            if (!affNode)
                assignment = prevAssignment;
            else
-                assignment = aff.assignPartitions(new GridAffinityFunctionContextImpl(sorted, prevAssignment,
-                    discoEvt, topVer, backups));
+                assignment = aff.assignPartitions(afCtxt);


Тут тоже

dream-x · 2017-07-24T11:26:46Z

...rc/main/java/org/apache/ignite/internal/processors/affinity/GridAffinityAssignmentCache.java

        }
        else
-            assignment = aff.assignPartitions(new GridAffinityFunctionContextImpl(sorted, prevAssignment, discoEvt,
-                topVer, backups));
+            assignment = aff.assignPartitions(afCtxt);


dream-x · 2017-07-24T11:50:06Z

...re/src/main/java/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunction.java

@@ -17,10 +17,6 @@

 package org.apache.ignite.cache.affinity.rendezvous;

-import java.io.Externalizable;


Почему это есть в мастере а у тебя удалено? Нужно вернуть.

dream-x · 2017-07-24T12:23:20Z

...rc/main/java/org/apache/ignite/internal/processors/affinity/GridAffinityAssignmentCache.java

+     * @param nodes Topology.
+     * @return Frequency distribution: counts of partitions on node.
+     */
+    private static List<List<Integer>> freqDistribution(List<List<ClusterNode>> partitionsByNodes,


почему статик?

dream-x · 2017-07-24T12:34:48Z

...a/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunctionSimpleBenchmark.java

@@ -349,14 +352,25 @@ private double chiSquare(List<List<Integer>> byNodes, int parts, double goldenNo
    }

    /**
+     * @throws Exception If failed.
+     */
+    public void testImprovedLog() throws Exception {


Однозначно написать тест проверки распределения.

dream-x · 2017-07-24T12:35:49Z

...a/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunctionSimpleBenchmark.java

+    /**
+     * @return Ignite Logger.
+     */
+    private IgniteLogger initializeIgniteLogger() {


убрать

dream-x · 2017-07-24T12:35:54Z

...a/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunctionSimpleBenchmark.java

+     * @param log Logger instance.
+     * @param ignite Ignite instance.
+     */
+    private AffinityFunction initializeAffinityFunction(boolean isOld, int parts, boolean exclNeighbors,


убрать

dream-x · 2017-07-24T12:36:03Z

...a/org/apache/ignite/cache/affinity/rendezvous/RendezvousAffinityFunctionSimpleBenchmark.java

        // TODO choose another affinity function to compare.
-        AffinityFunction aff1 = new RendezvousAffinityFunction(true, 256);
+        AffinityFunction aff0 = initializeAffinityFunction(false, 256, true, log, null);


вернуть

dream-x · 2017-07-24T17:13:58Z

modules/core/src/main/java/org/apache/ignite/IgniteSystemProperties.java

@@ -637,6 +642,8 @@
    public static final String IGNITE_CLIENT_CACHE_CHANGE_MESSAGE_TIMEOUT =
        "IGNITE_CLIENT_CACHE_CHANGE_MESSAGE_TIMEOUT";

+


зачем 2 пустых строки?

dream-x · 2017-07-24T17:14:40Z

...rc/main/java/org/apache/ignite/internal/processors/affinity/GridAffinityAssignmentCache.java

@@ -295,6 +298,8 @@ public void onReconnected() {

        assert assignment != null;

+        printDistribution(assignment, sorted, cacheOrGrpName, ctx.localNodeId().toString(), assignment.size());


ты передаешь assignment, зачем передавать assignment.size, ты же сможешь получить это

dream-x · 2017-07-28T12:44:35Z

...rc/main/java/org/apache/ignite/internal/processors/affinity/GridAffinityAssignmentCache.java

+        String ignitePartDistribution = System.getProperty(IgniteSystemProperties.IGNITE_PART_DISTRIBUTION_WARN_THRESHOLD);
+
+        if (Boolean.valueOf(ignitePartDistribution) || ignitePartDistribution == null) {
+            printDistribution(assignment, sorted, cacheOrGrpName, ctx.localNodeId().toString(), assignment.size());


тебе не нужно передавать assignment.size() он у тебя уже есть в assignment

dream-x · 2017-07-28T12:46:52Z

modules/core/src/main/java/org/apache/ignite/IgniteSystemProperties.java

@@ -414,6 +414,11 @@
    public static final String IGNITE_CACHE_CLIENT = "IGNITE_CACHE_CLIENT";

    /**
+     * Property controlling distribution calculation flag.
+     */
+    public static final String IGNITE_PART_DISTRIBUTION_WARN_THRESHOLD = "IGNITE_PART_DISTRIBUTION_WARN_THRESHOLD";


эта опция не флаг, прочитай внимательнее
Add system property IGNITE_PART_DISTRIBUTION_WARN_THRESHOLD with default value 0.1 to print warn message only when nodes count differs more then threshold;

dream-x · 2017-07-28T13:20:11Z

...gnite/cache/affinity/rendezvous/RendezvousAffinityFunctionCalculateDistributionSelfTest.java

+public class RendezvousAffinityFunctionCalculateDistributionSelfTest extends AbstractAffinityFunctionSelfTest {
+    /** Ignite. */
+    private static Ignite ignite;
+


Лучше переопределить метод afterTests что бы в нем останавливать ноды.

dream-x · 2017-07-28T13:20:54Z

...gnite/cache/affinity/rendezvous/RendezvousAffinityFunctionCalculateDistributionSelfTest.java

+     * @throws Exception If failed.
+     */
+    public void testDistributionCalculationEnabled() throws Exception {
+        System.setProperty(IgniteSystemProperties.IGNITE_PART_DISTRIBUTION_WARN_THRESHOLD, String.valueOf(true));


Как уже выше писал, здесь не булевое значение.

vopolski added 3 commits July 4, 2017 16:30

IGNITE-4756

30f3ae5

IGNITE-4756

be797db

IGNITE-4756

cc345de

dream-x reviewed Jul 4, 2017

View reviewed changes

IGNITE-4756

f4212b5

dream-x reviewed Jul 4, 2017

View reviewed changes

vopolski added 10 commits July 5, 2017 12:33

IGNITE-4756

dc2b935

IGNITE-4756

56a65ea

IGNITE-4756

1903caf

IGNITE-4756

7551f13

IGNITE-4756

b303fb5

IGNITE-4756

c9d6a33

IGNITE-4756

0d6ab3c

IGNITE-4756

08fbdcb

IGNITE-4756

4d31cfb

IGNITE-4756

2a0d095

Improved Java Doc and Code Style.

dream-x reviewed Jul 20, 2017

View reviewed changes

vopolski added 2 commits July 20, 2017 17:35

IGNITE-4756

96cba61

Improved Java Doc, Code Style and Abbreviation.

IGNITE-4756

8985596

Improved Java Doc, Code Style and Abbreviation.

vadopolski force-pushed the ignite-4756 branch from 0f60104 to 8985596 Compare July 24, 2017 11:46

deleted setter getter constructor

5402fa0

dream-x reviewed Jul 24, 2017

View reviewed changes

vopolski added 4 commits July 24, 2017 17:37

IGNITE-4756

a208694

IGNITE-4756

6cf30c9

IGNITE-4756

c15f7a8

IGNITE-4756

4583080

dream-x reviewed Jul 24, 2017

View reviewed changes

vopolski added 8 commits July 24, 2017 20:19

IGNITE-4756

9950758

IGNITE-4756

b05d959

IGNITE-4756

413889d

IGNITE-4756

ebcfda1

IGNITE-4756

4d0b362

IGNITE-4756

e0f0b12

IGNITE-4756

7fcd1ac

IGNITE-4756

d66512c

dream-x reviewed Jul 28, 2017

View reviewed changes

vopolski and others added 8 commits July 28, 2017 18:55

ignite-4756

aefb566

added topology starting

2fa4841

IGNITE-4756 Array and ListList solutions.

d728a62

ignite-4756 Array solution

6f11106

IGNITE-4756 deleted flag

aef11de

added tests with log

10622f9

ignite-4756

40547a0

IGNITE-4856

def3659


		AffinityFunction aff3 = new RendezvousAffinityFunction(true, 1024);

		for (int nodesCnt1 : nodesCnts1) {

		log.info("#### NODES:" + nodes + " nodes\n");

		for (List<Integer> byNode : byNodes) {

		@@ -280,21 +282,26 @@ public void onReconnected() {

		List<List<ClusterNode>> assignment;

		GridAffinityFunctionContextImpl afCtxt = new GridAffinityFunctionContextImpl(sorted, prevAssignment,

		@@ -17,10 +17,6 @@

		package org.apache.ignite.cache.affinity.rendezvous;

		import java.io.Externalizable;

		@@ -637,6 +642,8 @@
		public static final String IGNITE_CLIENT_CACHE_CHANGE_MESSAGE_TIMEOUT =
		"IGNITE_CLIENT_CACHE_CHANGE_MESSAGE_TIMEOUT";

		@@ -295,6 +298,8 @@ public void onReconnected() {

		assert assignment != null;

		printDistribution(assignment, sorted, cacheOrGrpName, ctx.localNodeId().toString(), assignment.size());

Ignite 4756 #3

Are you sure you want to change the base?

Ignite 4756 #3

Conversation

vadopolski commented Jul 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment